Catch agnostic HTTP error types in yarn and swap the SSL test#24281
Catch agnostic HTTP error types in yarn and swap the SSL test#24281mwdd146980 wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Validation Report
Run Passed validations (20)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa9b064670
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| HTTPStatusError, | ||
| HTTPInvalidURLError, | ||
| HTTPConnectionError, | ||
| HTTPSSLError, |
There was a problem hiding this comment.
When the check runs with the httpx2 backend and httpx raises a protocol/generic request failure, the wrapper maps it to datadog_checks.base.utils.http_exceptions.HTTPRequestError (for example LocalProtocolError in the base httpx2 tests). This tuple only adds the narrower status/invalid-url/connection/SSL classes, while the HTTPError name here is still requests.exceptions.HTTPError, so those backend-agnostic request errors bypass the yarn.can_connect CRITICAL service check and escape without the expected failure reporting.
Useful? React with 👍 / 👎.
| @@ -0,0 +1 @@ | |||
| Catch the backend-agnostic HTTP error types when querying the YARN ResourceManager API so the check stays correct after the httpx migration. | |||
There was a problem hiding this comment.
This entry describes restoring correct behavior after the httpx migration, but .changed entries are reserved for breaking/significant changes and bump the integration's major version per the repo instructions. Shipping this as changed can publish an unnecessary major YARN release; please use a .fixed entry for this patch-level compatibility fix.
Useful? React with 👍 / 👎.
1 participant
What does this PR do?
Widen the two HTTP exception handlers in
yarnto catch the backend-agnostichttp_exceptionstypes alongside the existingrequestsones, and swap the SSL verification test to inject and assert the agnostic type.Motivation
Part of the httpx migration. The agnostic
http_exceptionshierarchy shares no base class withrequests.exceptions, so the check must catch both families to keep working once the HTTP backend swaps to httpx. The loneTimeouthandler pairs withHTTPTimeoutError; the four-type tuple (HTTPError,InvalidURL,ConnectionError,SSLError) pairs withHTTPStatusError,HTTPInvalidURLError,HTTPConnectionError,HTTPSSLError. No observable behavior change under the shippedrequestsbackend.Verification
Full
yarnunit suite passes. Confirmed red-then-green ontest_ssl_verification: with the test swapped to injectHTTPSSLError(including its own localexcept HTTPSSLError:around the checked call, notpytest.raises) and production left requests-only, the test fails withAssertionError: Needed exactly 1 candidates for 'yarn.can_connect', got 0, since the un-widened production handler never fires the CRITICAL service check. After widening, it passes. TheTimeoutwidening has no test coverage either way and was verified by reading the handler, which mirrors the same log-and-reraise shape already proven by the SSL test.ddev test -fsis clean, and norequestsreference remains intests/test_yarn.pyexcept the retainedrequests_get_mockfixture helper name.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged